fix(hurl): fold not work if set foldexpr to `vim.treesitter.foldexpr()` #162

wenjinnn · 2024-05-23T10:12:50Z

Set filetype to hurl-nvim by default make vim.treesitter.foldexpr() not work, causes zc zo to fail in response buffer, it happens only on nvim 0.10. this commit check if edgy.nvim exists and then set filetype to hurl-nvim, otherwise just set filetype to input param type.

Also I was wondering if there have any way to set buf name safety with nui.nvim?

I tried to use vim.api.nvim_buf_set_name(split.bufnr, 'hurl-response') at split.show, but got a error when run a request second time, it seems like bufnr got wrong value or something. I would love to make a PR if I can fix it, right now the split result window have no name, maybe add a name will be nice.

Summary by CodeRabbit

Bug Fixes
- Improved buffer filetype handling, ensuring compatibility with edgy.nvim when available.

set filetype to `hurl-nvim` by default make `vim.treesitter.foldexpr()` not work, causes zc zo to fail. this commit check if edgy.nvim exists and then set filetype to hurl-nvim, otherwise just set filetype to input param type.

codesandbox · 2024-05-23T10:12:54Z

Review or Edit in CodeSandbox

Open the branch in Web Editor • VS Code • Insiders

Open Preview

changeset-bot · 2024-05-23T10:12:54Z

⚠️ No Changeset found

Latest commit: f2fb792

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

ellipsis-dev · 2024-05-23T10:12:54Z

Your free trial has expired. To keep using Ellipsis, sign up at https://app.ellipsis.dev for $20/seat/month or reach us at [email protected]

coderabbitai · 2024-05-23T10:12:58Z

Walkthrough

The recent changes in the lua/hurl/split.lua file focus on optimizing the buffer options setup within the Split and M.show functions. The buf_options declaration has been moved from the Split function to a conditional block in M.show, which now also includes logic to conditionally set the filetype using edgy.nvim if available, and then based on the type parameter.

Changes

File Path	Change Summary
`lua/hurl/split.lua`	Removed initial `buf_options` declaration from `Split` function. Added conditional block in `M.show` to set `filetype` using `edgy.nvim` if available, and then based on the `type` parameter.

🐰✨ In code we trust, through changes we grow,
Buffers now smarter, with options in tow.
Edgy and nimble, the filetypes align,
Lua scripts dancing, in harmony they shine.
Hurl’s split evolves, with logic so fine,
A toast to the coders, on this bright design! 🌟

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai generate interesting stats about this repository and render them as a table.
- @coderabbitai show all the console.log statements in this repository.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to full the review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

sweep-ai · 2024-05-23T10:14:00Z

Sweep: PR Review

Authors of pull request: @wenjinnn

lua/hurl/split.lua

Made the setting of the 'filetype' buffer option conditional based on the availability of 'edgy.nvim' and adjusted the timing of setting the filetype to the provided type parameter.

coderabbitai

Actionable comments posted: 1

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between ca3e78a and f2fb792.

Files selected for processing (1)

lua/hurl/split.lua (3 hunks)

Additional comments not posted (1)

lua/hurl/split.lua (1)

23-23: Safe use of pcall to check for edgy.nvim.

Using pcall here is a good practice as it prevents the application from crashing if edgy.nvim is not present or fails to load. This is a robust way to handle optional dependencies.

coderabbitai · 2024-05-23T10:14:30Z

lua/hurl/split.lua

+  if pcall(require, 'edgy') then
+    -- Create a custom filetype so that we can use https://github.com/folke/edgy.nvim to manage the window
+    -- E.g: { title = "Hurl Nvim", ft = "hurl-nvim" },
+    vim.api.nvim_buf_set_option(split.bufnr, 'filetype', 'hurl-nvim')
+  end
+
+  -- Set content to highlight, refer https://github.com/MunifTanjim/nui.nvim/issues/76#issuecomment-1001358770
+  vim.api.nvim_buf_set_option(split.bufnr, 'filetype', type)


Potential issue: The filetype is set twice when edgy.nvim is available.

The current logic sets the filetype to 'hurl-nvim' if edgy.nvim is available (line 26), but then it immediately sets it again to the type parameter (line 30). This could lead to unexpected behavior or override settings intended for 'hurl-nvim'. Consider restructuring the logic to ensure the filetype is set only once based on the conditions.

jellydn · 2024-05-23T15:35:58Z

Hi @wenjinnn

Also I was wondering if there have any way to set buf name safety with nui.nvim?

That's exactly what the author answered as below. The usage is correct.

buf_options = { filetype = 'hurl-nvim' },

MunifTanjim/nui.nvim#76 (comment)

wenjinnn · 2024-05-24T01:58:15Z

Hi @wenjinnn

Also I was wondering if there have any way to set buf name safety with nui.nvim?

That's exactly what the author answered as below. The usage is correct.
buf_options = { filetype = 'hurl-nvim' },
MunifTanjim/nui.nvim#76 (comment)

Thanks for reply @jellydn . I think this issue you refers to just explain this PR. set filetype to hurl-nvim make treesitter parser can't identify which language are in the response buffer (I guess we just can't have filetype 'hurl-nvim' and get treesitter highlight together), the reason that we got treesitter highlight from response buffer is we set filetype to particular language later in show function:

  -- Set content to highlight, refer https://github.com/MunifTanjim/nui.nvim/issues/76#issuecomment-1001358770
  vim.api.nvim_buf_set_option(split.bufnr, 'filetype', type)

Although we got treesitter highlight, but we can't get foldlevel calculate by vim.treesitter.foldexpr(), cause the foldlevel calculate only happens at first filetype setup (Not 100% sure, but this is the result of my test), my configuration was like:

vim.opt.foldmethod = "expr"
vim.opt.foldexpr = 'v:lua.vim.treesitter.foldexpr()'

Also I was wondering if there have any way to set buf name safety with nui.nvim?

And this is another problem I want to improve yet, just discuss in here. right now the response buffer have no name, it shows like:

I think give a buffer name like 'hurl-response' would be nice, please feel free to correct me.

jellydn · 2024-05-26T12:14:43Z

I think give a buffer name like 'hurl-response' would be nice, please feel free to correct me.

That sounds good @wenjinnn Let's do it. Thanks.

jellydn · 2024-05-28T03:56:45Z

Thanks @wenjinnn LGTM!

jellydn · 2024-05-28T03:59:36Z

One thing I noticed on this PR @wenjinnn the edgy integration won't work anymore.

        { title = "hurl.nvim", ft = "hurl-nvim", size = { width = 0.4 } },

wenjinnn · 2024-05-28T06:12:17Z

One thing I noticed on this PR @wenjinnn the edgy integration won't work anymore.
        { title = "hurl.nvim", ft = "hurl-nvim", size = { width = 0.4 } },

After we have buffer name, this setting could work with edgy.nvim, just a little wordy:

      left = {
        {
          ft = "json",
          title = "hurl-response",
        },
        {
          ft = "html",
          title = "hurl-response",
        },
        {
          ft = "text",
          title = "hurl-response",
        },
      },

I can update the README, and maybe remove all these lines at

hurl.nvim/lua/hurl/split.lua

Lines 22 to 27 in a185094

    
           -- If have edgy.nvim 
        
           if pcall(require, 'edgy') then 
        
             -- Create a custom filetype so that we can use https://github.com/folke/edgy.nvim to manage the window 
        
             -- E.g: { title = "Hurl Nvim", ft = "hurl-nvim" }, 
        
             vim.api.nvim_buf_set_option(split.bufnr, 'filetype', 'hurl-nvim') 
        
           end

, since it not work anymore.

jellydn · 2024-05-28T12:11:02Z

One thing I noticed on this PR @wenjinnn the edgy integration won't work anymore.
        { title = "hurl.nvim", ft = "hurl-nvim", size = { width = 0.4 } },
After we have buffer name, this setting could work with edgy.nvim, just a little wordy:
      left = {
        {
          ft = "json",
          title = "hurl-response",
        },
        {
          ft = "html",
          title = "hurl-response",
        },
        {
          ft = "text",
          title = "hurl-response",
        },
      },
I can update the README, and maybe remove all these lines at

hurl.nvim/lua/hurl/split.lua

Lines 22 to 27 in a185094

-- If have edgy.nvim

if pcall(require, 'edgy') then

-- Create a custom filetype so that we can use https://github.com/folke/edgy.nvim to manage the window

-- E.g: { title = "Hurl Nvim", ft = "hurl-nvim" },

vim.api.nvim_buf_set_option(split.bufnr, 'filetype', 'hurl-nvim')

end

, since it not work anymore.

Well, that won't work as well. The ft will move all json/html/text files to the left. Do you try with https://github.com/folke/edgy.nvim by above setting? @wenjinnn

jellydn · 2024-05-28T12:30:15Z

FYI - Fixed with my commit, here is the working config https://github.com/jellydn/lazy-nvim-ide/blob/main/lua/plugins/extras/hurl.lua

wenjinnn · 2024-05-28T12:47:59Z

One thing I noticed on this PR @wenjinnn the edgy integration won't work anymore.
        { title = "hurl.nvim", ft = "hurl-nvim", size = { width = 0.4 } },
After we have buffer name, this setting could work with edgy.nvim, just a little wordy:
      left = {
        {
          ft = "json",
          title = "hurl-response",
        },
        {
          ft = "html",
          title = "hurl-response",
        },
        {
          ft = "text",
          title = "hurl-response",
        },
      },
I can update the README, and maybe remove all these lines at

hurl.nvim/lua/hurl/split.lua

Lines 22 to 27 in a185094

-- If have edgy.nvim

if pcall(require, 'edgy') then

-- Create a custom filetype so that we can use https://github.com/folke/edgy.nvim to manage the window

-- E.g: { title = "Hurl Nvim", ft = "hurl-nvim" },

vim.api.nvim_buf_set_option(split.bufnr, 'filetype', 'hurl-nvim')

end

, since it not work anymore.
Well, that won't work as well. The ft will move all json/html/text files to the left. Do you try with https://github.com/folke/edgy.nvim by above setting? @wenjinnn

Oh sorry for that, I just try it once with edgy.nvim, cuz I don't really use it daily.
I have misunderstood the title setting, fortunately we can still achieve this with filter, I do some more test with it, now this should work well:

        {
          ft = "json",
          filter = function(buf)
            return vim.fn.bufname(buf) == "hurl-response"
          end,
        },
        {
          ft = "html",
          filter = function(buf)
            return vim.fn.bufname(buf) == "hurl-response"
          end,
        },
        {
          ft = "text",
          filter = function(buf)
            return vim.fn.bufname(buf) == "hurl-response"
          end,
        },

wenjinnn · 2024-05-28T13:25:44Z

FYI - Fixed with my commit, here is the working config https://github.com/jellydn/lazy-nvim-ide/blob/main/lua/plugins/extras/hurl.lua

This made fold level computing failed again. but I find a way to compatible with edgy.nvim and the treesitter foldexpr, I'll open another PR.

coderabbitai bot reviewed May 23, 2024

View reviewed changes

wenjinnn force-pushed the main branch from 689b5ec to f2fb792 Compare May 27, 2024 02:43

wenjinnn mentioned this pull request May 27, 2024

feat(hurl): add buffer name for split response window #163

Merged

7 tasks

jellydn merged commit 356194e into jellydn:main May 28, 2024
6 checks passed

wenjinnn mentioned this pull request May 28, 2024

fix(split): no fold find if foldexpr setup #166

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(hurl): fold not work if set foldexpr to `vim.treesitter.foldexpr()` #162

fix(hurl): fold not work if set foldexpr to `vim.treesitter.foldexpr()` #162

wenjinnn commented May 23, 2024 •

edited

Loading

codesandbox bot commented May 23, 2024

changeset-bot bot commented May 23, 2024 •

edited

Loading

ellipsis-dev bot commented May 23, 2024

coderabbitai bot commented May 23, 2024 •

edited

Loading

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (`.coderabbit.yaml`)

Documentation and Community

sweep-ai bot commented May 23, 2024 •

edited

Loading

coderabbitai bot left a comment

coderabbitai bot May 23, 2024

jellydn commented May 23, 2024

wenjinnn commented May 24, 2024 •

edited

Loading

jellydn commented May 26, 2024

jellydn commented May 28, 2024

jellydn commented May 28, 2024

wenjinnn commented May 28, 2024 •

edited

Loading

jellydn commented May 28, 2024 •

edited

Loading

jellydn commented May 28, 2024 •

edited

Loading

wenjinnn commented May 28, 2024

wenjinnn commented May 28, 2024

fix(hurl): fold not work if set foldexpr to vim.treesitter.foldexpr() #162

fix(hurl): fold not work if set foldexpr to vim.treesitter.foldexpr() #162

Conversation

wenjinnn commented May 23, 2024 • edited Loading

Summary by CodeRabbit

codesandbox bot commented May 23, 2024

Review or Edit in CodeSandbox

changeset-bot bot commented May 23, 2024 • edited Loading

⚠️ No Changeset found

ellipsis-dev bot commented May 23, 2024

coderabbitai bot commented May 23, 2024 • edited Loading

Walkthrough

Changes

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (.coderabbit.yaml)

Documentation and Community

sweep-ai bot commented May 23, 2024 • edited Loading

Sweep: PR Review

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot May 23, 2024

Choose a reason for hiding this comment

jellydn commented May 23, 2024

wenjinnn commented May 24, 2024 • edited Loading

jellydn commented May 26, 2024

jellydn commented May 28, 2024

jellydn commented May 28, 2024

wenjinnn commented May 28, 2024 • edited Loading

jellydn commented May 28, 2024 • edited Loading

jellydn commented May 28, 2024 • edited Loading

wenjinnn commented May 28, 2024

wenjinnn commented May 28, 2024

fix(hurl): fold not work if set foldexpr to `vim.treesitter.foldexpr()` #162

fix(hurl): fold not work if set foldexpr to `vim.treesitter.foldexpr()` #162

wenjinnn commented May 23, 2024 •

edited

Loading

changeset-bot bot commented May 23, 2024 •

edited

Loading

coderabbitai bot commented May 23, 2024 •

edited

Loading

CodeRabbit Configration File (`.coderabbit.yaml`)

sweep-ai bot commented May 23, 2024 •

edited

Loading

wenjinnn commented May 24, 2024 •

edited

Loading

wenjinnn commented May 28, 2024 •

edited

Loading

jellydn commented May 28, 2024 •

edited

Loading

jellydn commented May 28, 2024 •

edited

Loading